-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Ready] Fast Concurrent Deque Through Explicit Timestamping #127
base: integration
Are you sure you want to change the base?
Conversation
cтресс тесты не собирались из-за того, что несколько раз пересобирал разными версиями gcc. пересобрал с нуля одной версией - все заработало |
@EduardBlees - пока тесты не проходят для большей части архитектур |
@eugenyk #6 не собирается ассемблерный код под архитектуру i386, это поправим. #7 и #4 выдают ошибку Exception: Illegal. Причем ошибка не только в стресс, но и в юнит тестах. По ошибке абсолютно не понятно, чем она вызвана. Я могу только предположить: у нас ассемблерный код написан только для двух архитектур: ts_hardwaretimestamp.h. И тесты используют платформозависимый HardwareTimestamp класс. Можно тесты переписать на AtomicCounterTimestamp, который не зависит от платформы и компилятора. |
FYI - здесь собран зоопарк реализаций rdtsc для разных архитектур: |
@pr3sto по-хорошему надо бы в зависимости от платформы определять тип счётчика и для нереализованных действително использовать кроссплатформенный, через traits его передать в контейнер. Тут Максим меня может поправить |
|
@pr3sto, это ошибка вызова некорректной инструкции, ниже прилагаю содержимое core dump, полученного после запуска модульного теста - в общем ваше предположение верно в части счётчика
|
@eugenyk по этой ссылке есть только инструкция rdtsc. у нас используются: rdtsc и rdtscp. поэтому для других архитектур и компиляторов не добавил реализацию (для i386 поправил). libcds/test/unit/deque/ts_deque.cpp Line 80 in d5ece4c
|
@pr3sto, тест всё равно не проходит под i386 |
@eugenyk я не вижу другого выхода для нас, кроме как удалить поддержку i386. У нас нет возможности тестировать и отлаживать код для этой платформы, а работа вслепую, как видим, результатов не приносит. Тем более, что у нас написан платформонезависимый atomiccountertimestamp, который работает абсолютно также, да и от алгоритма взятия временных меток работа дека никак не зависит |
Если вы попросите доступ - у вас будет возможность отлаживать код и под этой архитектурой, это не проблема. В любом случае, поддерживаться должны все архитектуры, которые сейчас поддерживаются библиотекой - пусть и через платформонезависмый счётчик будет поддерживаться i386, должно собираться и работать |
@@ -22,6 +23,15 @@ namespace cds { namespace tshardwaretimestamp { | |||
return ret; | |||
} | |||
|
|||
static inline int has_rdtscp() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's quit strange to have "has_..." function returning logically boolean value with int signature
|
||
deque_type dq(1, 0); | ||
test( dq ); | ||
if (cds::tshardwaretimestamp::platform::has_rdtscp() == 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You check rdtscp existence only in unit tests, but what should do developers who will use your API, the same? It should be transparent for users
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Проблема в том, что, как я понял, нет возможности проверить наличие rdtscp во время компиляции. Эту проверку необходимо делать во время выполнения программы. Я не знаю, как сделать это прозрачным для пользователей. Подскажите?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's possible to determine it through cpuid
like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не совсем понимаю, что вы имеете ввиду. У нас и так проверка наличия rdtscp
происходит с помощью cpuid
. И функция cds::tshardwaretimestamp::platform::has_rdtscp()
возвращает 1, если процессор поддерживает rdtscp
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Всё верно, я сначала ответил на вопрос, а потом посмотрел реализацию
Implementation of [2014] Dodds,Haas,Kirsch Fast Concurrent Data-Structures Through Explicit Timestamping
Выполняли: Чирухин, Блеес (3304), Саклакова (3303)